[AHM] Add Polkadot call filtering #559
Conversation
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
relay/polkadot/src/lib.rs
Outdated
| // ParaSessionInfo has no calls | ||
| ParasDisputes(..) => (OFF, ON), // TODO check with security | ||
| ParasSlashing(..) => (OFF, ON), // TODO check with security | ||
| OnDemand(..) => (OFF, ON), |
There was a problem hiding this comment.
If we get paritytech/polkadot-sdk#5990 merged in time we should try to avoid filtering on-demand, as there will no longer be a balances dependency and we should avoid a situation where parachains can't make blocks through the migration.
relay/polkadot/src/lib.rs
Outdated
| Registrar(..) => (OFF, ON), | ||
| Slots(..) => (OFF, ON), // TODO not sure | ||
| Auctions(..) => (OFF, OFF), | ||
| Crowdloan(..) => (OFF, ON), // TODO maybe only payouts |
There was a problem hiding this comment.
Yea probably just withdraw, refund and dissolve.
Ideally we'd transfer this state to AH and allow people to withdraw or refund there directly, but it would be basically an entirely new pallet and would break all the crowdloan apps maybe unnecessarily. That's likely going to be the next phase of AHM though when the EDs go up. Still like 20 months until the last crowdloans expire so we need something long term
There was a problem hiding this comment.
The impact is very low, I would just disable.
There was a problem hiding this comment.
The impact is very low, I would just disable.
You mean also after the migration? I was thinking about enabling the withdraw function after the migration again.
There was a problem hiding this comment.
Changed to:
Crowdloan(crowdloan::Call::<Runtime>::dissolve { .. } | crowdloan::Call::<Runtime>::refund { .. } | crowdloan::Call::<Runtime>::withdraw { .. }) => (OFF, ON),
Crowdloan(..) => (OFF, OFF),
relay/polkadot/src/lib.rs
Outdated
| FastUnstake(..) => (OFF, OFF), | ||
| // DelegatedStaking has on calls | ||
| // ParachainsOrigin has no calls | ||
| Configuration(..) => (OFF, ON), |
There was a problem hiding this comment.
| Configuration(..) => (OFF, ON), | |
| Configuration(..) => (ON, ON), |
All root calls
There was a problem hiding this comment.
We also can have a similar impl for the EnsureOrigin if needed to let only Fellows to command some calls during migration. I do not think we can execute the root fast enough.
| let mut t: sp_io::TestExternalities = | ||
| frame_system::GenesisConfig::<T>::default().build_storage().unwrap().into(); | ||
|
|
||
| // MQ calls are never filtered: |
There was a problem hiding this comment.
Did we get to the bottom of the prioritisation of system messages? I lost track of the status of your streaking work
There was a problem hiding this comment.
Yea it worked here but we should get it audited & merged paritytech/polkadot-sdk#6059
| } | ||
|
|
||
| fn is_forbidden(call: &polkadot_runtime::RuntimeCall) -> bool { | ||
| let Err(err) = call.clone().dispatch(RuntimeOrigin::signed(AccountId32::from([0; 32]))) else { |
There was a problem hiding this comment.
This is actually a nice test to run on basically every pallet and extrinsic during the migration except for a few exceptions that are explicitly allowed from signed origins - the ones that are root only will fail anyway from this origin.
We could then have a separate check for the root calls that should be filtered, with the baseline being that all root calls are allowed except the ones we know will cause problems.
There was a problem hiding this comment.
Yes we could do that. Currently we need to manually construct the call arguments as well since our RuntimeCall enum does not implement an Arbitrary trait.
relay/polkadot/src/lib.rs
Outdated
| FastUnstake(..) => (OFF, OFF), | ||
| // DelegatedStaking has on calls | ||
| // ParachainsOrigin has no calls | ||
| Configuration(..) => (OFF, ON), |
There was a problem hiding this comment.
We also can have a similar impl for the EnsureOrigin if needed to let only Fellows to command some calls during migration. I do not think we can execute the root fast enough.
relay/polkadot/src/lib.rs
Outdated
| Registrar(..) => (OFF, ON), | ||
| Slots(..) => (OFF, ON), // TODO not sure | ||
| Auctions(..) => (OFF, OFF), | ||
| Crowdloan(..) => (OFF, ON), // TODO maybe only payouts |
There was a problem hiding this comment.
The impact is very low, I would just disable.
Co-authored-by: Dónal Murray <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
|
Going to merge. Tests should run after #561 |
To be merged into the AHM working branch.
Changes:
Configure the RC Migrator pallet as call filter for the Polkadot Relay chain
Test
Does not require a CHANGELOG entry